Skip to content

Fix: The button for toggling the left pane visibility in the launcher… #1962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 20, 2025

Conversation

SiriusXT
Copy link
Member

@SiriusXT SiriusXT commented May 17, 2025

1 and 2 are overridden in 4, and the logic in 4 is clearer.

  1. The button for toggling the left pane visibility in the launcher was initially not visible on startup
if (document.hasFocus() || this.currentLeftPaneVisible === true) {

This condition was added by me in #1905, but I can't recall the reason for adding it and I believe it's unnecessary. In any case, it causes a bug:

In the browser, if the left pane is hidden and then you switch to another application (causing the page to lose focus), and then click the refresh button in the browser, the toggle button in the launcher appears blank.

图片


  1. Fix (left pane toggle logic): properly handle both synced and current window visibility states

When options.is("leftPaneVisible") changes, there are two cases to handle:

Case 1: options and currentLeftPaneVisible are inconsistent.
In this case, we need to update currentLeftPaneVisible to match options, and then call refreshIcon.

Case 2: options and currentLeftPaneVisible are the same.
This can be triggered by the toggleLeftPaneCommand.
For example, both window 1 and window 2 have visible left panes. If the user hides the left pane in window 1 (setting options to false), and then triggers the toggle command in window 2, options becomes true again, matching currentLeftPaneVisible.
In this case, we need to manually invert this.currentLeftPaneVisible in window 2 to toggle the pane visibility.

Both cases can effectively be handled by inverting this.currentLeftPaneVisible.


  1. fix(ui): Exclude launcher pane width from Split calculation; Record Split state for multiple windows.
    and fix: Incorrectly calculating rest-pane width
    In vertical layout, the Split between leftPane and restPane needs to exclude the width of the launcherPane. Otherwise, the visible width of launcherPane will be compressed to less than --launcher-pane-size.

When multiple windows are open, if the leftPane is hidden and then shown again, its width should restore to the value before it was hidden in the current window, rather than retrieving it from Options. The same applies to the Split between center-pane and right-pane.


  1. fix(leftpaneVisibility): Make leftpane visibility switch no longer de…
    Changing the visibility of the left pane is a purely frontend event that does not require any interaction with the backend. The option:leftPaneVisible only needs to store the most recent visibility state.

Copy link

github-actions bot commented May 17, 2025

Folder/File Previous size New size Difference
/upload/TriliumNextNotes-Server-1962-merge-linux-x64.tar.xz 68.69MB +68.69MB (+100.00%)
TOTAL +68.69MB

@SiriusXT SiriusXT marked this pull request as ready for review May 18, 2025 08:53
@SiriusXT SiriusXT marked this pull request as draft May 18, 2025 09:41
@SiriusXT SiriusXT marked this pull request as ready for review May 18, 2025 10:20
@SiriusXT SiriusXT marked this pull request as draft May 19, 2025 15:44
@SiriusXT SiriusXT marked this pull request as ready for review May 19, 2025 16:37
@SiriusXT SiriusXT marked this pull request as draft May 20, 2025 03:46
@SiriusXT SiriusXT marked this pull request as ready for review May 20, 2025 07:19
@eliandoran eliandoran added this to the v0.94.0 milestone May 20, 2025
@eliandoran eliandoran merged commit 82242a8 into develop May 20, 2025
11 checks passed
@eliandoran eliandoran deleted the left-pane branch May 20, 2025 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants